-
Notifications
You must be signed in to change notification settings - Fork 24.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mute bwc tests for old versions in FIPS JVMs #32839
Conversation
Elasticsearch versions earlier than 6.4.0 cannot properly run in a FIPS 140 JVM. This commit ensures that we do not try to run bwc tests that entail spinning up < 6.4.0 nodes when CI is run in a FIPS 140 JVM. It also reverts e497173 and e64bb48 as the workarounds inserted there are no longer required. Resolves elastic#32727
Pinging @elastic/es-security |
List<Version> indexCompatibleVersions = bwcVersions.indexCompatible | ||
// Versions before 6.4.0 cannot be run in a FIPS 140 JVM, so exclude them | ||
if (inFipsJvm){ | ||
indexCompatibleVersions.removeAll { it.before("6.4.0")} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer this not to be repeated in the build logic and encapsulated in VersionCollection
, perhaps by passing inFipsJvm
to the constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When calling the constructor in build.gradle
, the inFipsJvm
property hasn't been evaluated, as this happens in BuildPlugin#globalBuildInfo()
.
Another approach would be to add a inFipsJvm
optional param to getSnapshotsIndexCompatible()
, getIndexCompatible()
etc, defaulting to false. WDYT?
Just a quick note: I think the "Resolves" reference int eh description might be wrong as it links to an issue that seems unrelated? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this makes sense as something VersionCollection figures out. Whether a particular version will run on a fips JVM has to do with that version, not version compatibility, which is what VersionCollection is all about. Instead of doing this on each compatible list method, I think you can do it as an attribute on the Version object itself? Then the code in eg full cluster restart tests can be something like:
for (Version version : bwcVersions.wireCompatible) {
if (inFipsJvm && version.isFipsCompatible() == false) {
continue
}
....
}
Elasticsearch versions earlier than 6.4.0 cannot properly run in a
FIPS 140 JVM. This commit ensures that we do not try to run bwc
tests that entail spinning up < 6.4.0 nodes when CI is run in a FIPS
140 JVM.
It also reverts e497173 and e64bb48 as the workarounds inserted there
are no longer required.
Resolves #32737